Skip to content

Comments

Fix transposition problem with small ASCII renderer#645

Merged
Shane32 merged 1 commit intomasterfrom
microtranspose
Oct 6, 2025
Merged

Fix transposition problem with small ASCII renderer#645
Shane32 merged 1 commit intomasterfrom
microtranspose

Conversation

@Shane32
Copy link
Owner

@Shane32 Shane32 commented Oct 6, 2025 •
edited
Loading

This fixes the output of the ASCII 'small' renderer so it is correct instead of transposed along the x/y diagonal.

See note in description of #592

Summary by CodeRabbit

  • Bug Fixes

    • Corrected ASCII QR code rendering to ensure accurate orientation and pixel placement. Output now matches expected visuals across small QR codes, inverted mode, quiet-zone variations, and custom end-of-line settings. No API changes.

  • Tests

    • Updated expected ASCII QR code outputs in tests to reflect the corrected rendering behavior.

@Shane32 Shane32 added this to the 1.7.0 milestone Oct 6, 2025
@Shane32 Shane32 self-assigned this Oct 6, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 6, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

Transposed module access in ASCIIQRCode.GetGraphicSmall by swapping row/column indices when reading moduleData. Updated unit tests’ expected ASCII outputs to match the new rendering. No public APIs changed.

Changes

Cohort / File(s) Summary
ASCII QR renderer core
QRCoder/ASCIIQRCode.cs		 In GetGraphicSmall, swapped row/column indexing when reading moduleData for current and next pixels; loop structure and palette logic unchanged.
ASCII renderer tests
QRCoderTests/AsciiQRCodeRendererTests.cs		 Updated expected ASCII QR code string literals across tests to align with the transposed rendering; no test logic changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • gfoidl

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes the main change by stating that it fixes a transposition issue in the small ASCII renderer, directly reflecting the swap of row/column indices in the code. It clearly identifies the problem being addressed without unnecessary detail or jargon. This phrasing is specific enough that a teammate scanning PR history will immediately understand the core fix being applied.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch microtranspose

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Shane32 Shane32 requested a review from gfoidl October 6, 2025 06:37
@Shane32 Shane32 mentioned this pull request Oct 6, 2025
Merged
@Shane32 Shane32 merged commit b6c6a06 into master Oct 6, 2025
10 checks passed
@Shane32 Shane32 deleted the microtranspose branch October 6, 2025 11:57
@Shane32 Shane32 added the bug Bug or bug fix label Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gfoidl gfoidl gfoidl approved these changes

Assignees

@Shane32 Shane32

Labels

bug Bug or bug fix

Projects

None yet

Milestone

1.7.0

Development

Successfully merging this pull request may close these issues.

2 participants

@Shane32 @gfoidl